-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow setting of CIRCUITPY_DISPLAY_LIMIT in settings.toml #10158
base: 9.2.x
Are you sure you want to change the base?
Conversation
It looks like boards with built-in displays don't like pointer references to the display structures, I was unsuccessful getting a pointer/array combination of variables to work so I think I'll either have to look at the way the boards with builtin displays are accessing the structures or consider leaving the compile time portion of the array in place and just access the dynamically allocated array elements for display indexes > (compile time)CIRCUITPY_DISPLAY_LIMIT. Fun for another night..... |
I suspect this is because the table of attributes for board is in ROM IIRC. That means it can't be updated on the fly. I'm not sure what a better approach is. |
I ended up dynamically allocating memory for any displays beyond those defined with the compile time parameter and then used some c macros to select either the compile time or dynamic storage based on the display index. I'm thinking it probably wouldn't be too difficult to move from this version to one that simply allocated the space as new displays were defined but figured small steps.... I'd still be curious if you think allocating the memory in the main.c program is appropriate 😁 |
I tried testing with an I2C display (BUSDISPLAY) and a DVI display (FRAMEBUFFERIO). It seemed like the I2C display output was usually getting lost when the DVI was primary, I could get the I2C to display if I did a displayio.release_displays() and then ctrl-D. The most recent commit attempts to do a better job of determining if one of the display structure elements is being used by an active display although I don't understand the mechanism that sets the object type element so there's probably a better solution. I thought this version worked better but after some more testing I'm not sure it made any difference. There seems to be a somewhat random factor, perhaps the contents of newly allocated memory that the display/bus structures are using. |
Well.... After banging at the code for a bit I decided to test the I2C display I'm using with the current version (no PR) of CircuitPython and it behaves in almost the same way as the PR so the odd behavior seems to be a quirk of the I2C display or my soldering. I'm using a featherwing doubler with a Feather RP2350 & a FeatherWing OLED. So now I'm thinking the last commit should be dropped although I don't think it's doing any harm so I'll leave it for now. |
Ya, it is fine. |
Requires possible future PR to enable settings.toml parameter to allow sharing of TFT_DC/RST pins: import time
import board
import displayio
import fourwire
import terminalio
from adafruit_display_text import label
from adafruit_st7789 import ST7789
displayio.release_displays()
spi = board.SPI()
tft_dc = board.D10
tft_cs = [board.D5,board.D13,board.D11,board.D12]
display_bus = []
display = []
digit = []
color = [0xff0000,0x00ff00,0x0000ff,0xffff00]
splash = []
for i in range(4):
display_bus.append(fourwire.FourWire(spi, command=tft_dc, chip_select=tft_cs[i]))
display.append(ST7789(display_bus[i], rotation=180, width=135, height=240, rowstart=40, colstart=53))
digit.append(label.Label(terminalio.FONT, text="0", scale=10, color=color[i], x=45, y=80))
splash.append(displayio.Group())
splash[i].append(digit[i])
display[i].root_group=splash[i]
sec1=0
timer=time.time()
while True:
while time.time() == timer:
pass
timer=time.time()
sec1+=1
digit[3].text=str((sec1%60)%10)
digit[2].text=str((sec1%60)//10)
digit[1].text=str((sec1//60)%10)
digit[0].text=str((sec1//60)//10) |
I believe the ctrl-D safemode crash has been resolved thanks to a planning comment by tannewt 😁 I left the shared SPI pin (actually only the DC pin) hack in this version to help with testing but will pull them out before the final version. If anyone needs to share the RST pin to test, let me know.... I haven't been able to get set_primary_display (supervisor.runtime.display=display) to work yet and I suspect the logic for setting/determining the "primary_display_number" isn't working either. But that's another nights problem.... |
DC and RST pins are shared across the 4 displays on the T-Keyboard-S3. The stock firmware that ships with it configures the I'm not sure if leaving the hack will cause regressions elsewhere in CircuitPython, but I would like to have that included in this PR. |
I believe Scott was worried that allowing the pins to be shared would not let people know if they chose pins being used for other functions. If/when this PR gets merged, I plan to submit a second one that would enable the sharing of the pins by setting a second settings.toml parameter. That way the user protection Scott was looking for would be in place by default, but if an advanced user wanted to override it they would be able to. Edit: presumably for a board like the T-Keyboard-S3 the feature could be enabled by default as well. |
I think this is mostly working except for the "Set Primary Display" (supervisor.runtime.display=display) but either I misunderstand what that function does or it's also not working on current builds for anything but dvi displays. In either 9.2.x or my artifacts, If I try to set the primary display manually the display is not identified as active and the following somewhat misleading message is displayed
Based on my reading of the code, I'm assuming the "primary display" mechanism was meant to allow any of the possibly multiple displays to be designated as "primary" and survive VM restarts. If that is indeed the case this PR still needs work to get it implemented. Currently I think only the display which populates the first The current behavior seems workable to me (although I can see that it would be nice to have a working primary display mechanism) but I think at this point I need some feedback as to whether I should try and get primary display working in 9.2.x./main and then come back to this PR. |
For the user protection side of things, I agree with this so long as it doesn't limit board design. There aren't many examples of multiple displays, and there's even fewer examples of shared DC and RST pins. My guess is that while it works, it's sub-optimal. But... it might also be that creators haven't ported CircuitPython to their multi-display projects yet. I think it deserves a bit of consideration given how many people are looking forward to having this amazing new feature.
This sounds like a good compromise, especially if a switch to Zephyr in 10.x or 11.x renders this obsolete. Would you please make that a compile-time flag too? All 4 displays are built-in, and I'm working on getting them to start on boot. Being able to set that in the build would help a lot. Thanks again for your work on this! |
Set the maximum supported number of displayio displays. This value overrides the CIRCUITPY_DISPLAY_LIMIT compile-time flag.
I do have some questions about this solution.
I'm not sure if this fixes #1760 since Tannewt has said he hopes to implement a better solution in CP 11. I'm also not sure main.c was the right location to insert the call to malloc_display_memory but I don't know displayio well enough to know if there was a better place somewhere in the displayio modules.
So far I've only been testing with the RP2350 feather (DVI display) with a TFT featherwing (SPI display). I'll go ahead and get some more configurations set up for testing but figured I get this posted so others could start reviewing.